Skip to content

feat: add downloadBaseURL, checksum, and downloadAuthToken inputs (closes Azure#206)#268

Open
benjaminbob21 wants to merge 1 commit into
Azure:mainfrom
benjaminbob21:feat/custom-mirror
Open

feat: add downloadBaseURL, checksum, and downloadAuthToken inputs (closes Azure#206)#268
benjaminbob21 wants to merge 1 commit into
Azure:mainfrom
benjaminbob21:feat/custom-mirror

Conversation

@benjaminbob21

@benjaminbob21 benjaminbob21 commented Jun 1, 2026

Copy link
Copy Markdown

Add downloadBaseURL, checksum, and downloadAuthToken inputs (closes #206)

Supports private kubectl mirrors (air-gapped / enterprise) with optional SHA256 verification and optional bearer-token auth for private artifact repositories. Default base URL is unchanged.

Security: https-only, blocks loopback/link-local/RFC1918 hosts, re-validates each redirect hop, strips the Authorization header on cross-origin redirects, masks the token via core.setSecret, and validates version format before any URL/path use.

@benjaminbob21 benjaminbob21 marked this pull request as ready for review June 1, 2026 18:41
@benjaminbob21 benjaminbob21 requested a review from a team as a code owner June 1, 2026 18:41
@benjaminbob21 benjaminbob21 changed the title feat: add downloadBaseURL and checksum inputs (closes Azure#206) feat: add downloadBaseURL, checksum, and downloadAuthToken inputs (closes Azure#206) Jun 2, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds support for downloading kubectl from custom/private mirrors by introducing a configurable download base URL, optional SHA256 verification, and optional bearer-token authentication. This extends the action’s download pipeline with additional URL validation and a “secure download” path intended to mitigate SSRF/token leakage risks when using non-default mirrors.

Changes:

  • Add new action inputs: downloadBaseURL, checksum, downloadAuthToken, and route downloads through mirror-aware logic.
  • Implement URL/version validation and a redirect-aware secureDownload() path for custom mirrors, plus optional SHA256 verification.
  • Expand unit and integration tests to cover custom mirrors, redirects/SSRF guards, and checksum validation.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/run.ts Plumbs new inputs into version resolution and download/caching, adds checksum verification and token handling.
src/run.test.ts Adds extensive test coverage for custom mirror behavior, URL/version validation, redirects, auth header stripping, and checksum checks.
src/helpers.ts Introduces secureDownload, base URL/version validation helpers, and baseURL-aware URL construction for downloads/version discovery.
package.json Adds @actions/http-client dependency used by secureDownload.
package-lock.json Locks the newly added @actions/http-client dependency.
action.yml Documents and exposes the new inputs to action consumers.
.github/workflows/integration-tests.yml Adds an integration test validating checksum success and failure behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/helpers.ts
Comment on lines +77 to +80
if (status !== 200) {
response.message.resume()
throw new toolCache.HTTPError(status)
}
Comment thread src/run.ts
Comment on lines +131 to +138
// Scope the cache key per mirror so a binary cached from one source isn't silently reused for another.
const cacheKey = isDefaultBaseURL(baseURL)
? version
: `${version}-${crypto
.createHash('sha256')
.update(normalizeBaseURL(baseURL))
.digest('hex')
.slice(0, 12)}`
Comment thread src/helpers.ts
Comment on lines +96 to +100
const chunks: Buffer[] = []
let received = 0
for await (const chunk of response.message as AsyncIterable<Buffer>) {
received += chunk.length
if (received > SECURE_DOWNLOAD_MAX_BYTES) {
Comment thread src/run.ts
}
}

if (version.toLocaleLowerCase() === 'latest') {
@Tatsinnit

Copy link
Copy Markdown
Member

Security review on the new custom mirror path:

I found two places where this should fail closed rather than warn/allow.

  1. Custom mirrors should require checksum.
    Right now a non-default downloadBaseURL can still download and cache an unverified kubectl binary when checksum is omitted. On self-hosted runners that creates a cache-poisoning path if the mirror or network is compromised.

  2. SSRF defense should cover DNS-resolved private targets, not just literal IPs.
    The current validation blocks literal loopback/link-local/RFC1918 addresses, but a hostname that resolves to 169.254.169.254 or another private address would still be fetched. The hardened path should resolve the hostname before each custom-mirror request/redirect hop and reject private or link-local results.

I validated both behaviors locally with targeted tests:

  • custom mirror without checksum is rejected
  • hostnames resolving to private/link-local IPs are rejected
  • existing auth-header stripping and redirect tests still pass

Focused validation result: npx vitest run src/run.test.ts -> 90/90 passing.

@Tatsinnit

Tatsinnit commented Jun 15, 2026

Copy link
Copy Markdown
Member

Suggested change summary for a follow-up patch:

Please do make sure that these changes make sense, if there are checks in place which will avoid scenario then feel free to avoid these changes.

  • In run():

    • trim version before checking for latest
    • reject any non-default downloadBaseURL unless checksum is provided
    • keep the existing downloadAuthToken restriction for the default public mirror
  • In downloadKubectl():

    • fail closed for custom mirrors without checksum, even if the function is called directly
    • preserve the existing per-mirror cache keying and per-run checksum verification
  • In the custom download path (secureDownload()):

    • validate the resolved hostname before each request hop, not just the literal URL host
    • reject hostnames that resolve to loopback, link-local, RFC1918, ULA, or metadata-service ranges
    • keep the existing redirect re-validation and cross-origin auth-header stripping
  • In tests:

    • add a case proving a DNS hostname resolving to 169.254.169.254 is rejected
    • change custom-mirror tests to always pass a checksum when the flow is expected to succeed
    • add a case proving custom mirror + missing checksum fails immediately

@Tatsinnit Tatsinnit left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this PR should change before merge for two security reasons:

  1. A custom downloadBaseURL currently allows downloading and caching kubectl without a required checksum.
    This should fail closed, not warn. On self-hosted runners, an unverified custom-mirror binary can poison the tool cache and persist across later jobs.

  2. The SSRF guard only blocks literal private/link-local IPs.
    A hostname that resolves to 169.254.169.254, RFC1918 space, loopback, or similar internal ranges would still be fetched today. The custom download path should validate DNS-resolved addresses on each request/redirect hop and reject blocked targets.

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature Request: Add downloadBaseURL input

3 participants